Profile form fixes - #1119
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refines profile updating, trainer filtering, and search indexing to prevent duplicate records, correct filter options, and optimize Solr reindexing.
- Adds a profile ID check to user updates to block creation of new profile records
- Fixes language select options and conditional display of trainer filters
- Refactors Solr reindex callbacks from individual models into a single
after_commitinProfile
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/controllers/users_controller.rb | Added check_profile_id before_action to forbid mismatched IDs |
| app/views/users/_form.html.erb | Updated scraper-email checkbox and replaced language select input |
| app/models/profile.rb & trainer.rb | Removed per-model callbacks; unified reindex_trainer callback |
| app/models/concerns/searchable.rb | Revised public filtering logic for Trainer/Profile searches |
| app/helpers/search_helper.rb | Enhanced facet_title to gracefully handle blank language names |
| config/locales/en.yml | Added translations for scraper emails and profile language label |
| test/test_helper.rb | Stubbed external HTTP requests to example.com |
| test/fixtures/users.yml | Introduced admin_trainer fixture for admin tests |
| test/fixtures/profiles.yml | Added admin_trainer_profile fixture |
| test/controllers/users_controller_test.rb | Updated profile update tests and added forbidden-ID test |
| test/controllers/trainers_controller_test.rb | Adjusted expected trainer count and inclusion assertion |
Comments suppressed due to low confidence (2)
config/locales/en.yml:974
- These keys live under
en.useranden.profile, but SimpleForm will look undersimple_form.labels.user.check_broken_scrapersandsimple_form.labels.profile.language. Consider nesting these translations undersimple_form.labelsto match SimpleForm’s lookup path.
check_broken_scrapers: Receive emails with scrapers that have not found anything last period.
app/views/users/_form.html.erb:40
- The variable
user_fisn’t defined in this form; it looks like you intended to usef.inputfor the user builder. Replaceuser_f.inputwithf.inputor properly defineuser_fviafields_for.
<%= user_f.input :check_broken_scrapers %>
| end | ||
|
|
||
| if attribute_method?(:public) && !user&.is_admin? # Find a better way of checking this | ||
| if name == 'Trainer' || name == 'Profile' |
There was a problem hiding this comment.
[nitpick] This branch forces all searches for Trainer/Profile to only show public records, even for admins. Document this behavior or refine the condition so that admins can still see private items when intended.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Motivation and context
User noticed trainer language select on profile edit page was showing Yes/No options instead of languages, ran into other issues whilst debugging.
Checklist
to license it to the TeSS codebase under the
BSD license.